Skip to content

fix(types): make RowKind short string parsing case-insensitive#15

Open
slfan1989 wants to merge 1 commit into
apache:mainfrom
slfan1989:paimon-cpp-14
Open

fix(types): make RowKind short string parsing case-insensitive#15
slfan1989 wants to merge 1 commit into
apache:mainfrom
slfan1989:paimon-cpp-14

Conversation

@slfan1989
Copy link
Copy Markdown
Contributor

@slfan1989 slfan1989 commented May 26, 2026

Purpose

Linked issue: close #14

Make RowKind::FromShortString consistent with Java Paimon's RowKind.fromShortString behavior.

Java Paimon normalizes the input to uppercase before matching, so lowercase short strings such as +i, -u, +u, and -d are accepted. This PR updates paimon-cpp to accept the same lowercase variants.

Tests

Added unit test coverage for lowercase RowKind short strings:

  • +i maps to INSERT
  • -u maps to UPDATE_BEFORE
  • +u maps to UPDATE_AFTER
  • -d maps to DELETE

API and Format

No.
This change does not affect public API headers under include/, storage format, or protocol.

Documentation

No.
This is a compatibility bug fix and does not introduce a new feature.

Generative AI tooling

@slfan1989
Copy link
Copy Markdown
Contributor Author

slfan1989 commented May 26, 2026

@zjw1111 @leaves12138 Could we accept this small change? Thank you very much!

@lxy-9602
Copy link
Copy Markdown
Contributor

lxy-9602 commented May 26, 2026

Thank you for the contribution. This fix looks correct and addresses the case-sensitivity issue.

We have a utility function, StringUtils::ToUpperCase, that has not been merged yet. Would it be possible to wait until that utility is available? I think it could make the code a bit cleaner. We expect to merge it soon, and then sync it in this PR if that works for you. Thanks!

@slfan1989
Copy link
Copy Markdown
Contributor Author

slfan1989 commented May 26, 2026

Thank you for the contribution. This fix looks correct and addresses the case-sensitivity issue.

We have a utility function, StringUtils::ToUpperCase, that has not been merged yet. Would it be possible to wait until that utility is available? I think it could make the code a bit cleaner. We expect to merge it soon, and then sync it in this PR if that works for you. Thanks!

@lxy-9602 Thank you for the feedback. That sounds good to me.

I’m happy to wait until StringUtils::ToUpperCase is merged, and then I can update this PR to use the utility function to make the code cleaner.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] RowKind::FromShortString should be case-insensitive

2 participants

@slfan1989 @lxy-9602